-
Notifications
You must be signed in to change notification settings - Fork 1k
Stm32 ethernet lib #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
…al. UDP is functional. Signed-off-by: fpr <[email protected]>
…nfiguration on network. Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
|
||
Circuit: | ||
* Ethernet shield attached to pins 10, 11, 12, 13 | ||
* STM32 board with Ethernet support | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add history modification below
Circuit: | ||
* Ethernet shield attached to pins 10, 11, 12, 13 | ||
* STM32 board with Ethernet support | ||
created 18 Dec 2009 | ||
by David A. Mellis | ||
modified 9 Apr 2012 | ||
by Tom Igoe | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Workaround: | ||
* If you can't send a message after the server has accepted the connection, | ||
reject the 26 first bytes. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this message is not very clear. What is the exact issue and why do we reject 26 bytes ?
|
||
created 12 April 2011 | ||
modified 9 Apr 2012 | ||
by Tom Igoe | ||
modified 02 Sept 2015 | ||
by Arturo Guadalupi | ||
|
||
Workaround: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for enabling Ethernet !
I have shared few generic comments and questions
libraries/NativeEthernet/README.adoc
Outdated
|
||
An Idle task is required to handle timer and data reception. Be careful to not | ||
lock the system in a loop where LwIP could never be updated. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how / where is the idel task implemented ?
libraries/NativeEthernet/README.md
Outdated
|
||
MAC address is defined in `stm32f4xx_hal_conf.h` with MAC_ADDR0, MAC_ADDR1,... | ||
Modify these defines to set your custom MAC address. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so how is MAC address defined ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mac address is set directly from the Ethernet class calling the function begin().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More precisely, where should user configure the MAC address - can you point to the code where this should be done ? this needs to be explained in the README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MAC address is defined in the sketch. I refer you to the examples for more details.
I kept the Arduino method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I will look at the examples - thx
@@ -47,41 +47,6 @@ | |||
#ifndef __LWIPOPTS_H__ | |||
#define __LWIPOPTS_H__ | |||
|
|||
#define LWIP_DEBUG LWIP_DBG_ON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the commit message should explain why you are removing the debug options
/* UDP structure */ | ||
struct udp_struct { | ||
struct udp_pcb *pcb; /* pointer on the current udp_pcb */ | ||
struct pbuf_data data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure that this is related to this commit "rename library ..."
shouldn't this be part of previous one ?
@@ -173,5 +180,5 @@ bool EthernetClient::operator==(const EthernetClient& rhs) { | |||
} | |||
|
|||
uint8_t EthernetClient::getSocketNumber() { | |||
return _sock; | |||
return 0; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does getSocketNumber always return 0 - is this a limitation of the implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a limitation because getSocketNumber() is not an Arduino function but implemented for the W5100 architecture. I just keep it for compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok.
cores/arduino/stm32/ethernet.h
Outdated
/* Exported macro ------------------------------------------------------------*/ | ||
/* Exported functions ------------------------------------------------------- */ | ||
|
||
__weak void stm32_eth_scheduler(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explain also how this is supposed to work, where is this function finally defined in the end?
@@ -128,9 +128,11 @@ void HAL_ETH_MspInit(ETH_HandleTypeDef *heth) | |||
} | |||
} | |||
|
|||
#ifdef ETH_INPUT_USE_IT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is ETH_INPUT_USE_IT introduced here ? Is it working both with Interrupts and without interrupts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it works with and without. But it is slower than the polling mode. And LwIP stack documentation recommends to use the polling mode.
It gives to the user the possibility to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks. This is I think an explanation that is needed in the commit message, or in comments where this switch is activated.
libraries/Ethernet/src/Ethernet.cpp
Outdated
@@ -52,6 +52,7 @@ void EthernetClass::begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dn | |||
void EthernetClass::begin(uint8_t *mac, IPAddress local_ip, IPAddress dns_server, IPAddress gateway, IPAddress subnet) | |||
{ | |||
stm32_eth_init(mac, local_ip.raw_address(), gateway.raw_address(), subnet.raw_address()); | |||
stm32_DHCP_manual_config(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is manual DHCP needed.
Generally speaking, commit messages need to explain why not what.
@@ -45,6 +45,9 @@ int main( void ) | |||
|
|||
for (;;) | |||
{ | |||
// Define by Ethernet library. It is defined as __weak. | |||
stm32_eth_scheduler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why doing it here in the core rather than in the loop ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the idle task for handle timer and data reception. It must be called as much as possible.
I put it here to be sure it is called at least one time per loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same would happen if you put this call in the loop, so this does not answer my question.
Also this seems here that it will be called for each loop for a board that supports it, even if I don't use Ethernet , is that right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is defined as weak in ethernet.c, when the library NativeEthernet is not included, this function is empty.
And to call it in the loop, we must create a function that the user should call (this function will not be refered in the Arduino documentation).
I thought it was easier to leave that in background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - this is maybe acceptable if this has no impact when the library is not included. This may be explained as well in the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this break if freertos or the arduino Scheduler library is used in combination with this library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know because I didn't study the LwIP stack in case of an "operating system". The NativeEthernet is oriented single threaded core. I refer you to the LwIP wiki or freertos example using the LwIP stack.
Maybe call stm32_eth_scheduler() inside its own thread or process is enough.
cores/arduino/stm32/pinmap.c
Outdated
@@ -83,3 +83,11 @@ uint32_t pinmap_merge(uint32_t a, uint32_t b) { | |||
// error("pinmap mis-match"); | |||
return (uint32_t)NC; | |||
} | |||
|
|||
PinName pin_pinName(const PinMap* map) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this pinmap function needed when adding Ethernet ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have called map->pin directly but I preferred to create a new function. The code is more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I think this should just be a commit by itself as not directly related to Ethernet.
Server validated. Code cleaned. Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
and increase execution speed. Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
…during compilation Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Signed-off-by: fpr <[email protected]>
Required comments added. Documentation updated. Signed-off-by: fpr <[email protected]>
6f94a9b
to
80b70aa
Compare
Possibility to use IT mode for ethernet input but it is better to keep the polling mode. Signed-off-by: fpr <[email protected]>
There seems to be a trend in this core of using #defines to deal with special situations. However, making this type of change with the Arduino IDE is less than obvious. Adding and changing flags to the boards.txt file is problematic for most people. The bigger problem with this approach is that any changes the user makes will be overwritten when the Board Manager provides an update to the core . I urge you to think of alternative ways to make conditional code work with the Arduino core. I'm talking about defines like this: |
you're right @RickKimball, |
I think you could take advantage of the recipe.hooks feature of the platform.txt file to conditionally override defines and gcc command line arguments on a per sketch basis. This seems to be a fairly new feature (since Arduino 1.6.something) something they added since I last looked to accomplish something like this. You might approach it like this: In the stm32 tools directory create a new tool called 'cpif' which copies a file called gcc_options.txt into the build directory if the file exists in the sketch directory or copies /dev/null if it doesn't
In platform.txt you add a recipe hook that is called before starting the build process.
modify the board.txt to use the @file feature of gcc or just add it to the platform.txt for all boards
When the user wants to override defines and gcc command line arguments they create a file called gcc_options.txt in the sketch directory. Here I imagine I want to override the F_CPU value
When I compile it uses the sketch specific defines and overrides.
Of course you would have to make this work with the different OS types. You would also have to deal with finding the users preference sketch home directory. I just hard coded it in the example above to use my $HOME/Arduino directory. Seems doable (I actually have this working ) if you care to open yourself to all kinds of user problems when they do bad things : ) |
Thx @RickKimball I will try. It is not a specific part for the Ethernet lib. It is more global. So I open a specific issue for this new feature. |
Hi @fprwi6labs,
|
See #52 , LwIP#1 and STM32Ethernet#1 |
NativeEthernet library added to allow STM32 boards with Ethernet feature to connect to the internet.
More information here.
NUCLEO-F429ZI is ready to use the NativeEthernet library.